Skip to content
This repository was archived by the owner on Mar 13, 2026. It is now read-only.

feat: drop read_only on a connection returned back to a pool#189

Merged
vi3k6i5 merged 11 commits intomainfrom
reset_conn
Jan 28, 2022
Merged

feat: drop read_only on a connection returned back to a pool#189
vi3k6i5 merged 11 commits intomainfrom
reset_conn

Conversation

@IlyaFaer
Copy link
Copy Markdown

@IlyaFaer IlyaFaer commented Jan 10, 2022

Current connections reset mechanism doesn't drop read_only property of a connection returned back to a connections pool. It makes the following cases possible:

  • user initiates a ReadOnly connection, executes some operations and returnes the connection back to the pool (automatically)
  • user initiates the next connection, without making it ReadOnly, but the pool returns the connection, which was toggled to the ReadOnly mode earlier (as it's a pool, it returns the connections created earlier, it is its purpos).

Such a situation can be very unexpected and will require for user to write additional logic to understand if the connection can be used in their case. To avoid it, we better drop read_only attribute to its default value when a connection is returned back to pool.

P.S. because of the same reasons we're already dropping staleness attribute on a connection reset.

@IlyaFaer IlyaFaer added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Jan 10, 2022
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. label Jan 10, 2022
Comment thread google/cloud/sqlalchemy_spanner/sqlalchemy_spanner.py
Comment thread test/test_suite.py
Comment thread test/test_suite.py
@IlyaFaer IlyaFaer requested a review from larkee January 10, 2022 09:18
@IlyaFaer IlyaFaer marked this pull request as ready for review January 10, 2022 09:18
@IlyaFaer IlyaFaer requested a review from a team January 10, 2022 09:18
@IlyaFaer IlyaFaer requested review from vi3k6i5 and removed request for larkee January 28, 2022 08:49
Copy link
Copy Markdown
Contributor

@vi3k6i5 vi3k6i5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vi3k6i5 vi3k6i5 merged commit 16388c1 into main Jan 28, 2022
@vi3k6i5 vi3k6i5 deleted the reset_conn branch January 28, 2022 10:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants